-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More tests #191
More tests #191
Conversation
550a1c7
to
502472f
Compare
Hmm, seems like fsck.erofs is not big endian safe, at least not the version in ubuntu. |
502472f
to
7999a6e
Compare
i_mtime_nsec is 32bit, so use lcfs_u32_to_file, not lcfs_u64_to_file. The later was breaking on big endian arches. Signed-off-by: Alexander Larsson <[email protected]>
This allows you to build an image with user xattrs, but not get things like e.g. selinux contexts. Signed-off-by: Alexander Larsson <[email protected]>
Like --skip-xattrs, but keeps user xattrs. Signed-off-by: Alexander Larsson <[email protected]>
We don't want to rewrite trusted.overlay to user.overlay anymore, as that was a leftover from when using the fuse fs combined with overlayfs, instead we need to support xattr escapes. Also, until fuse supports non-user xattrs, don't return them from the fuse APIs, because listing the xattr will work, but the getattr will then be filtered out and get ENODATA. Signed-off-by: Alexander Larsson <[email protected]>
This gets rid of the 00-ff files we generate Signed-off-by: Alexander Larsson <[email protected]>
Signed-off-by: Alexander Larsson <[email protected]>
Signed-off-by: Alexander Larsson <[email protected]>
Signed-off-by: Alexander Larsson <[email protected]>
Pass in $VALGRIND_PREFIX separately rather than via $BINDIR. This means we don't have to valgrind everything in $BINDIR, which will be useful later. Signed-off-by: Alexander Larsson <[email protected]>
These options tweak the output of dumpdir, similar to what we expect some composefs tools to do. In particular: --userxattrs: Only dump user.* xattrs. This is what composefs-fuse will do. --noescaped: Don't dump escaped xattrs. This is what a real composefs mount will do if kernel overlayfs doesn't support xattr escapes. --whiteouts: Convert regular whiteouts to xattr-based whiteouts. This is what mkcomposefs does to whiteouts. Signed-off-by: Alexander Larsson <[email protected]>
Signed-off-by: Alexander Larsson <[email protected]>
Generates 10 random images and runs some test on them. The tests run are: * Dump (parse and re-write it) image and esure we get identical results * Run fsck.erofs on image * Mount image using fuse and ensure dumpdir is the same (sans non-user-xattr and whiteout) * Run mkcomposefs on the fuse mount and ensure the result is similar. (It produces same fuse mount dump, but due to non-user xattrs and whiteouts its not producing an identical composefs image) Signed-off-by: Alexander Larsson <[email protected]>
7999a6e
to
2c365e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks sane, but I didn't deep dive into the tests or the xattr changes.
@@ -881,7 +881,7 @@ static int write_erofs_inode_data(struct lcfs_ctx_s *ctx, struct lcfs_node_s *no | |||
i.i_uid = lcfs_u32_to_file(node->inode.st_uid); | |||
i.i_gid = lcfs_u32_to_file(node->inode.st_gid); | |||
i.i_mtime = lcfs_u64_to_file(node->inode.st_mtim_sec); | |||
i.i_mtime_nsec = lcfs_u64_to_file(node->inode.st_mtim_nsec); | |||
i.i_mtime_nsec = lcfs_u32_to_file(node->inode.st_mtim_nsec); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, good old C automatic integer promotion rules, here to blow off our feet.
So I tested this, and it finds the problem:
diff --git a/libcomposefs/lcfs-internal.h b/libcomposefs/lcfs-internal.h
index f9cab47..749dbe6 100644
--- a/libcomposefs/lcfs-internal.h
+++ b/libcomposefs/lcfs-internal.h
@@ -70,10 +70,10 @@ static inline uint32_t lcfs_u32_to_file(uint32_t val)
return htole32(val);
}
-static inline uint64_t lcfs_u64_to_file(uint64_t val)
-{
- return htole64(val);
-}
+#define lcfs_u64_to_file(v) ({ \
+ _Static_assert(sizeof(v) == sizeof(uint64_t)); \
+ htole64(v); \
+})
static inline uint16_t lcfs_u16_from_file(uint16_t val)
{
Opinions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may hit problematic cases where we e.g. pass an int to u64_to_file(), but I'm fine with having those explicitly add a cast as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lemme try it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,195 @@ | |||
#!/usr/bin/python3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This repository is now up to 3 distinct languages (C, shell script, and Python). Personally, I would vote that eventually of using Python and shell script for tests we use Rust. I've had decent success with this, see e.g. https://github.com/ostreedev/ostree/blob/878d601665fd15781adf774d99dd86e9a172cb92/tests/inst/src/destructive.rs#L477 shows how Rust's macros can let one execute shell commands (xshell crate) with near-equal ergonomics to bash, but more safety than Python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love rust for production code, but I'm not sure how well it would fit with code that is a bit more throwaway and like tests. Seems like it may raise the bar for people to add new testcases. I'd be willing to be convinced though.
I don't quite have an environment to test big-endian... I know I could setup a QEMU environment, but honestly currently I don't have enough time on this now.. |
This adds some fusa/tools tweaks and then some initial work on randomized image testing.
For now it only uses fuse so we can run this during unit tests, but I want to follow up with similar integration tests that do an actual kernel mount.